Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix error handling for automatically derived version #814

Merged
merged 4 commits into from
Nov 20, 2019
Merged

Fix error handling for automatically derived version #814

merged 4 commits into from
Nov 20, 2019

Conversation

drawlerr
Copy link
Contributor

Let the source of the problem get to the user.

Closes #813

@drawlerr drawlerr added the bug Something's wrong label Nov 12, 2019
@drawlerr drawlerr added this to the 1.4.0 milestone Nov 12, 2019
@ebadyano
Copy link
Contributor

@drawlerr Thank you for opening a PR! With your change we now get this error:

[ERROR] Cannot race. Error in race control (ConnectionError(<urllib3.connection.HTTPConnection object at 0x105b30f60>: Failed to establish a new connection: [Errno 61] Connection refused) caused by: NewConnectionError(<urllib3.connection.HTTPConnection object at 0x105b30f60>: Failed to establish a new connection: [Errno 61] Connection refused))

Do we want to add something more user friendly, like: Could not connect to cluster at specified address, please check your connection. Or something along those lines?

@danielmitterdorfer danielmitterdorfer added the :Benchmark Candidate Management Anything affecting how Rally sets up Elasticsearch label Nov 15, 2019
danielmitterdorfer
danielmitterdorfer requested changes Nov 18, 2019
View reviewed changes
Copy link
Member

@danielmitterdorfer danielmitterdorfer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. I left a couple of questions.

esrally/racecontrol.py Outdated Show resolved Hide resolved
danielmitterdorfer
danielmitterdorfer approved these changes Nov 20, 2019
View reviewed changes
Copy link
Member

@danielmitterdorfer danielmitterdorfer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for iterating! LGTM

@drawlerr drawlerr merged commit 541fdfc into elastic:master Nov 20, 2019
@drawlerr drawlerr deleted the fix-version-check branch November 20, 2019 23:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danielmitterdorfer danielmitterdorfer danielmitterdorfer approved these changes

@dliappis dliappis Awaiting requested review from dliappis

@ebadyano ebadyano Awaiting requested review from ebadyano

Assignees
No one assigned
Labels
:Benchmark Candidate Management Anything affecting how Rally sets up Elasticsearch bug Something's wrong
Projects
None yet
Milestone
1.4.0
Development

Successfully merging this pull request may close these issues.

Version check error message is misleading
3 participants
@drawlerr @ebadyano @danielmitterdorfer